-
Notifications
You must be signed in to change notification settings - Fork 1.3k
web: add web-app server for development and production builds #20126
Conversation
9e5f5a0
to
5400089
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work 🎊
I would be worried a bit about people messing with sourcegraph.com, but maybe that's not a reasonable fear :) I think we should not make it serve on sourcegraph.test:3443, though because it's too easy to think it's a safe dev environment.
- A static mock is used instead of the JS context returned from the SOURCEGRAPH_API_URL.
I think that should be implemented soon. The flags set change a lot about the app behavior and it is also dynamic in the backend so if I change a config, the jscontext would change if it was a real deployed app.
- index.html template is different from what we have in cmd/frontend/internal/app/ui/app.html. We should reuse it to be consistent with the default production setup.
Definitely agree. Also it's so easy to forget about this file when someone updates the app.html
file.
@@ -0,0 +1,65 @@ | |||
import 'dotenv/config' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we already have quite a few packages, I think this should be it's own package under client/dev-server
or whatever. That way, this package will still be only code required in the webapp, and it's not as easy to accidentally import webpack and such into the final web bundle (accidentally imported from this file, for example). Also the dev server needs commonjs as a target, and the webapp can be esnext, so maybe another indicator that a proper package with it's own tsconfig.json file might make sense here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the right call. I thought about doing it in this PR. Still, apart from this change, we need to extract shared Webpack configuration into a standalone package to avoid adding loaders to three different configs: Web, Browser, and Storybook. I believe this step should be done before extracting client/dev-server
because it might result in duplicated work and circular dependencies between web
and dev-server
packages. First, move configs to a shared package. Then extract dev-server and use shared configs.
Co-authored-by: Erik Seliger <erikseliger@me.com>
I'm really excited to enable web dev without needing any backend services. |
Something we may want to consider is injecting some kind of banner somewhere "THIS USES PRODUCTION DATA FROM sourcegraph.com" because it may be easy to forget if you have multiple or old tabs open given this runs on localhost. I feel like it may otherwise only be a matter of time until someone accidentally misconfigures an external service on dotcom or something |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After following the READM
Updates
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! What matters more for security here (please add anything I didn't think of!):
- Makes devs lives easier ✨
- Uses k8s.sgdev as default
- Alerts users of which instance they're connected to
- No HTTPS yet because of upstream bug
This PR introduces an independent server for the web application, which can use any deployed API instance as a backend.
Independent web app server
This PR introduces an independent server for the web application which can use any deployed API instance as a backend.
Context
While working on the CSS modules configuration, I needed a way to iterate quickly on the
webpack.config.js
and test changes in the production environment: runyarn build-web
and check how CSS modules are loaded in the application. There was no easy/quick way to do it. @umpox found out that it's possible to change a couple of variables in thestart.sh
script to serve the production build. But this solution was quite slow because of the backend services involved.To solve this, I've configured a minimum viable server to serve web app production artifacts without our backend services. I've added HtmlWebpackPlugin to our
webpack.config.js
to produceindex.html
and configured Webpack dev server proxy to point the server to the deployed API instance. It allowed testing changes in production build without backend services quickly. This PR is the enhanced version of this setup.How it works
WEBPACK_SERVE_INDEX=true
. This plugin outputsindex.html
to theui/assets
.SOURCEGRAPH_API_URL
provided in the.env
file using the http-proxy-middleware.CSRF token is invalid
API error CSRF token is retrieved from theSOURCEGRAPH_API_URL
before the server starts. Then this value is used for every subsequent request to the API.sgs
cookie, all cookies from the API requests are propagated to the local client.Known issues
The issues listed below can be addressed in separate PRs. They are non-blocking for the functionality this PR introduces and doesn't affect existing workflows. We can iterate on each individual issue once we have the foundation in place.
site-config.json
is used instead of the JS context returned from theSOURCEGRAPH_API_URL
.gulpfile.js
. They should be shared with it after migration to Typescript.index.html
template is different from what we have incmd/frontend/internal/app/ui/app.html
. We should reuse it to be consistent with the default production setup.Changes
client/web/package.json
:yarn serve:dev
andyarn serve:prod
.dev
directory is added to theclient/web
package. It contains the server configuration used by the commands above..env.example
is added to theclient/web
to showcase the example env configuration.require('ts-node').register()
is added to thegulpfile.js
to allow TS usage forhtml-webpack-plugin
configuration.Updates
sg
CLI tool:sg run web-standalone
sg run enterprise-web-standalone
sg run web-standalone-prod
sg run enterprise-web-standalone-prod
.env.example
anddotenv
package becauseenv
variables are now defined insg.config.yaml
.